Re-naming dxy to lxy in L1TrackNtupleMaker and including a trk_charge branch #302
Conversation
| if (layerdisk >= N_LAYER && (!isPSmodule)) | ||
| layerdisk += N_DISK; | ||
| layerdisk += (N_LAYER - 1); |
There was a problem hiding this comment.
Why this change? Was original line a bug?
There was a problem hiding this comment.
I'm not sure how this got changed. I'll change this back.
| @@ -1089,7 +1085,7 @@ namespace trklet { | |||
| //Overlap size for the overlap phi bins in DR | |||
| double phiOverlapSize_{M_PI / 360}; | |||
| //The maximum number of tracks that are compared to all the other tracks per rinv bin | |||
| int numTracksComparedPerBin_{9999}; | |||
| int numTracksComparedPerBin_{32}; | |||
There was a problem hiding this comment.
Original code allowed 9999 tracks compared, and you reduced this to 32. Does this by itself explain the drop in tracking efficiency that causes git CI to fail for HYBRID?
There was a problem hiding this comment.
It's definitely possible, but we can't leave it at 9999, can we?
There was a problem hiding this comment.
I set it to 9999 when the DR simulation was buggy. Since the FW uses 32, and we want the DR simulation with kill option to roughly resemble the FW, the natural thing is to set it equal to 32. Does this only affect the "kill" DR or also the "merge" one?
There was a problem hiding this comment.
I forgot to change this when I switched to merging. I locally ran the merging version of the code (only HYBRID) with 1000 events over the same sample as the CI, and it doesn't seem like anything was affected from switching to 32 CM.
| @@ -158,8 +164,8 @@ void PurgeDuplicate::execute(std::vector<Track>& outputtracks, unsigned int iSec | |||
| // Best Guess: L1L2 > L1D1 > L2L3 > L2D1 > D1D2 > L3L4 > L5L6 > D3D4 | |||
| // Best Rank: L1L2 > L3L4 > D3D4 > D1D2 > L2L3 > L2D1 > L5L6 > L1D1 | |||
| // Rank-Informed Guess: L1L2 > L3L4 > L1D1 > L2L3 > L2D1 > D1D2 > L5L6 > D3D4 | |||
| const unsigned int curSeed = aTrack->seedIndex(); | |||
| static const std::vector<int> ranks{1, 5, 2, 7, 4, 3, 8, 6}; | |||
| unsigned int curSeed = aTrack->seedIndex(); | |||
There was a problem hiding this comment.
Why have you removed the const flag here?
| const unsigned int curSeed = aTrack->seedIndex(); | ||
| static const std::vector<int> ranks{1, 5, 2, 7, 4, 3, 8, 6}; | ||
| unsigned int curSeed = aTrack->seedIndex(); | ||
| std::vector<int> ranks{1, 5, 2, 7, 4, 3, 8, 6}; |
There was a problem hiding this comment.
Why did you remove static const here?
| unsigned int itrk) const { | ||
| for (unsigned int stcount = 0; stcount < stubsTrk.size(); stcount++) { | ||
| int i = stubsTrk[stcount].first; // layer/disk | ||
| bool barrel = (i > 0 && i < 10); |
There was a problem hiding this comment.
Magic numbers like 10 are not allowed in CMSSW
There was a problem hiding this comment.
I feel like we've had this discussion on these lines before. From what I can see, the 10 is used because the disk layers are +/- 11-15. I can change 10 in the first line tobarrel = ( i > 0 && i <= N_LAYER) and then in the second line to endcapA = i > N_LAYER and endcapB = i < 0. However, the (i - 1) and (i - 5) terms in the line that sets lay are there specifically to encode the layers to 0-15.
|
The title and description of this PR state that it is modifying L1TrackNtupleMaker, but most of the code changes affect the DR. Please change the title & description to reflect more accurately what this PR is for. |
…d a trk_charge and matchtrk_charge branch to L1TrackNtupleMaker
dally96
force-pushed
the
dally_L1TrackNtupleVars
branch
from
fd5c3b9 to
47b6576
Compare
I hope it's alright, but I changed this PR to no longer have modified DR. It should just be modifications on top of the current central development branch. |
PR description:
In L1TrackNtupleMaker, dxy and d0 are both variable names where dxy is defined as xy-distance of the PV from the interaction point, and d0 is defined as the transverse impact parameter. In displaced vertex studies, both dxy and d0 mean the same thing: the transverse impact parameter. I suggest dxy be changed to lxy in order to avoid confusion.
Adding trk_charge as a branch is just a matter of convenience since that information is lost in the ntuple.